-
Notifications
You must be signed in to change notification settings - Fork 27
Adding Libxc support #521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Adding Libxc support #521
Conversation
Added the LibXC integration files.
|
@susilehtola @Tirtho13. Are you satisfied with the LibXC tests which you have now included in MRChem? Could we consider ticking those two off as well? |
src/mrdft/Factory.cpp
Outdated
| coefs = {1.0}; | ||
| return; | ||
| } else if (name == "SVWN5") { | ||
| } else if (name == "SVWN5" || name == "LDA") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LDA is a bad keyword.
src/mrdft/Functional.cpp
Outdated
| /** @brief Run a collection of grid points through Libxc or XCFun | ||
| * | ||
| * Each row corresponds to one grid point. | ||
| * | ||
| * param[in] inp_data Matrix of input values | ||
| * param[out] out_data Matrix of output values | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was the comment removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The policy in MRChem is to include this documentation in the header files and no longer in the implementation files. This is why that has been removed. Same for all the other ones.
| * efficient to have the two consecutive points in two consecutive adresses in memory | ||
| * | ||
| * param[in] inp_data Matrix of input values | ||
| * param[out] out_data Matrix of output values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
src/mrdft/Functional.cpp
Outdated
| /** @brief Run a collection of grid points through Libxc or XCFun | ||
| * | ||
| * Each row corresponds to one grid point. | ||
| * | ||
| * param[in] inp_data Matrix of input values | ||
| * param[out] out_data Matrix of output values | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| /** @brief Contract a collection of grid points | ||
| * | ||
| * Each row corresponds to one grid point. | ||
| * | ||
| * param[in] xc_data Matrix of functional partial derivative values | ||
| * param[in] d_data Matrix of density input values | ||
| * param[out] out_data Matrix of contracted output values | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| /** @brief Contract a collection of grid points | ||
| * | ||
| * Each column corresponds to one set of grid points. | ||
| * | ||
| * param[in] xc_data Matrix of functional partial derivative values | ||
| * param[in] d_data Matrix of density input values | ||
| * param[out] out_data Matrix of contracted output values | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
|
|
||
| /** @brief Evaluates XC functional and derivatives for a given NodeIndex | ||
| * | ||
| * The electronic densities (total/alpha/beta) are given as input. | ||
| * The values of the zero order densities and their gradient are sent to xcfun. | ||
| * The output of xcfun must then be combined ("contract") with the gradients | ||
| * of the higher order densities. | ||
| * | ||
| * XCFunctional output (with k=1 and explicit derivatives): | ||
| * | ||
| * LDA: \f$ \left(F_{xc}, \frac{\partial F_{xc}}{\partial \rho}\right) \f$ | ||
| * | ||
| * GGA: \f$ \left(F_{xc}, | ||
| * \frac{\partial F_{xc}}{\partial \rho}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_x}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_y}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_z}\right) \f$ | ||
| * | ||
| * Spin LDA: \f$ \left(F_{xc}, \frac{\partial F_{xc}}{\partial \rho^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho^\beta}\right) \f$ | ||
| * | ||
| * Spin GGA: \f$ \left(F_{xc}, | ||
| * \frac{\partial F_{xc}}{\partial \rho^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho^\beta}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_x^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_y^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_z^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_x^\beta}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_y^\beta}, | ||
| * \frac{\partial F_{xc}}{\partial \rho_z^\beta} | ||
| * \right) \f$ | ||
| * | ||
| * XCFunctional output (with k=1 and gamma-type derivatives): | ||
| * | ||
| * GGA: \f$ \left(F_{xc}, | ||
| * \frac{\partial F_{xc}}{\partial \rho}, | ||
| * \frac{\partial F_{xc}}{\partial \gamma} \f$ | ||
| * | ||
| * Spin GGA: \f$ \left(F_{xc}, | ||
| * \frac{\partial F_{xc}}{\partial \rho^\alpha}, | ||
| * \frac{\partial F_{xc}}{\partial \rho^\beta }, | ||
| * \frac{\partial F_{xc}}{\partial \gamma^{\alpha \alpha}}, | ||
| * \frac{\partial F_{xc}}{\partial \gamma^{\alpha \beta }}, | ||
| * \frac{\partial F_{xc}}{\partial \gamma^{\beta \beta }} | ||
| * \right) \f$ | ||
| * | ||
| * param[in] inp Input values | ||
| * param[out] xcNodes Output values | ||
| * | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| coefs = {1.0, 1.0}; | ||
| return; | ||
| } else if (name == "B3LYP") { | ||
| ids = {XC_HYB_GGA_XC_B3LYP5}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not B3LYP!!!
| coefs = {1.0, 1.0}; | ||
| return; | ||
| } else if (name == "B3LYP") { | ||
| ids = {XC_HYB_GGA_XC_B3LYP5}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ids = {XC_HYB_GGA_XC_B3LYP5}; | |
| ids = {XC_HYB_GGA_XC_B3LYP}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we need to agree on what kind of "behavior" we want the code to have. Ideally a given functional keyword (let's take B3LYP as an example) should yield the same result regardless of the dft library used. Now it is a bit unfortunate that what libxc and xcfun define as vanilla B3LYP is not the same functional (same for e.g. PBE)
Given that libxc is by far more distributed, I would say we should follow the libxc convention and have some translation mechanism in place for xcfun. The drawback from my point of view is that this will break compatibility with previous results. But I guess we need to live with that.
tests/h2_scf_lda_libxc/h2.inp
Outdated
| } | ||
|
|
||
| WaveFunction { | ||
| method = LDA # Wave function method (HF or DFT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"LDA" is ambigous, unless it is exchange-only. Use a proper functional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LDA had become a (psudo) standard alias for SVWN5 which mrchem (and orca, pyscf, etc) use, this pr should not be the one to change this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where?
Orca lists
HFS # Hartree-Fock Slater (Slater exchange only)
LSD # Local spin density (VWN-5A form)
VWN5 # Local spin density (VWN-5)
VWN3 # Local spin density (VWN-3)
PWLDA # Local spin density (PW-LDA)
There is no "LDA".
PySCF with LDA gives
XC functionals = lda
P. A. M. Dirac., Math. Proc. Cambridge Philos. Soc. 26, 376 (1930)
F. Bloch., Z. Phys. 57, 545 (1929)
which is just the exchange functional, like I said above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK so Orca has documented this in the 6.1 manual; I looked at the 6.0 manual that does not have an LDA keyword.
But in PySCF LDA means something else.
There is no generally accepted meaning, which is why it should not be a keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t mind changing the keyword in the new tests, but there are other tests that use “lda”, and I don’t think it’s a good idea to remove the possibility of calling «lda» for svwn5 in mrchem as people might expect that to work (at least not in this pr)
|
Personally, I think we have a mergeable code now. It can run libxc on input demand, we have tests, the numbers are consistent and there is otherwise little change to the program, which was the goal here. There are still things to do on the libxc/dft side of the program, eg. restructuring the functional classes, fixing b3lyp, ... which can be done at a later point (it would also be good to not have too much happening in this pr considering that the commits will be smashed). Unless there is any specific comments, I suggest we wait for the pr to be reviewed :) |



Libxc interface in mrchem
This is a detailed list of things to do in the integration of Libxc into MRChem. The checklist should have all the necessary changes to complete the integration process.
This is the checklist of all the necessary changes.
NOTE: Please refrain from making commits to the checklists before the changes are made public for review. For owning a particular job, note your name at the end of the specified job. If you want to make a note, add it to the dev notes.
CMAKE files
Runtime backend
Libxc input
Density threshold
To do during/after restructuring (to avoid doing it twice)
Making functionals class abstract and create new classes for libxc/xcfun inheriting from functionals.
Ylva will make a rough separation of classes and then we can update a to do list of which parts that should be done (a lot of the functional class functions are using xcfun and should be made general, but that can be done function by function).
Libxc and functional citation
Get hyb parameters from libxc
Overwrite Libxc functional settings
Testing
Documentation